Skip to content

feat(srt): check srt-slurm recipes against master images before sweep dispatch / feat(srt):在派发 sweep 前检查 srt-slurm 配方与主配置镜像是否一致 - #3624

Open
chunfangamd wants to merge 2 commits into
mainfrom
chun/srt-recipe-preflight
Open

chunfangamd wants to merge 2 commits into
mainfrom
chun/srt-recipe-preflight

Conversation

@chunfangamd

@chunfangamd chunfangamd commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #3567; the two PRs belong together. #3567 aligns the two srt-slurm recipes whose container had drifted from the master-config image. This PR adds the check that would have caught the drift: each sweep now binds every planned srt-slurm point to the recipe variants it would launch, and stops before dispatching any job if a point doesn't bind.

Problem. An srt-slurm point names its images in two places: the master config's image (plus PREFILL_IMAGE for TileRT) and the containers in the recipe it launches. An image bump has to edit both, as #3446 did, but nothing compares them before benchmark jobs start.

#3567 first added a pytest scan for this. Review found that it couldn't gate run-sweep, compared image sets per recipe instead of the variant each point selects, skipped EVAL_CONFIG_FILE, and pinned checked-in config against the AGENTS.md test rules. #3567 dropped the scan, and this PR replaces it.

Changes

  1. Validator. infx/srt_slurm/preflight.py (python -m infx.srt_slurm.preflight) reads the matrix on stdin. If every srt-slurm point binds, it writes the matrix unchanged to stdout; otherwise it prints each problem once with the points it affects and exits 1.
    • Single-node: it builds the environment that benchmark-tmpl.yml exports for the point and calls the runtime's own select_recipe, which must match exactly one variant on engine, model, image, precision, parallelism, GPU count, speculative decoding, concurrency and KV offloading. The check runs per point, so a recipe shared by keys with different images is checked against each key's image.
    • Multi-node: a point must set CONFIG_FILE, unless it is eval-only and sets EVAL_CONFIG_FILE. That is the recipe lanes.config_file launches; any other point would fail only on the GPU runner. The validator expands every CONFIG_FILE and EVAL_CONFIG_FILE a point references with srtctl's own override expansion. A reference that selects no variant is reported, because srtctl would submit nothing. In every variant:
      • model.container and each roles.<role>.container must be the point's image (in either registry spelling) or a container alias that configs/runners.yaml defines for the runner's cluster. A TileRT point must set PREFILL_IMAGE, and its prefill role must be exactly that image, the only prefill name the launcher stages.
      • A TileRT frontend.container_image or benchmark.container_image must be the decode image (or an alias) or PREFILL_IMAGE.
      • A declared identity.container.image must name the point's image.
    • Benchmark clients: in single-node and multi-node variants alike, a benchmark.container_image that names another tag of the point's image is reported, because srtctl would pull that literal instead of the staged image. An unrelated client image is allowed, and other frontends may pin an image of their own: the ATOM agentic recipes pin rocm/atom-dev@sha256:f00a… for their atomesh frontend.
    • Paths: a single-node srt-recipe must resolve inside benchmarks/single_node/srt-slurm-recipes, and a multi-node reference must be a recipes/ path that resolves inside benchmarks/multi_node/srt-slurm-recipes, so .. and absolute paths are rejected.
    • Runner inventory: it is read only when the matrix has multi-node points. If this tooling's schema cannot read it, the validator says so and exits 1.
  2. Workflows.
    • run-sweep.yml: setup initializes the srt-slurm submodule and runs the validator after benchmark_schema --plan, before ci_priority. If the validator fails, setup fails, so canary-select and the benchmark jobs, which all require a successful setup, never start.
    • e2e-tests.yml (which trusted-external-sweep.yml and claude.yml also dispatch) and profile.yml: the validator and its srtctl come from the trusted tooling tree (PRIORITY_ROOT and its own srt-slurm submodule), and the measured tree is read only as data (--root "$MEASURED_ROOT"). The step always runs. require_launcher, earlier in the same step, already rejects revisions older than the Python launcher, and those are also the revisions whose runners.yaml this tooling cannot read.
  3. Tests. infx/tests/srt_slurm/test_preflight.py has 36 cases with small temporary recipes and runner inventories, per the AGENTS.md test rules. They cover:
    • a one-sided image update through the CLI, and a runner inventory read only for multi-node points;
    • throughput and eval-only points with neither recipe, only EVAL_CONFIG_FILE, or only CONFIG_FILE;
    • a variant swap in a shared recipe, and an EVAL_CONFIG_FILE mismatch;
    • a recipe that selects no variant unless :base is named, and a variant without model.container;
    • configured and misspelled aliases, registry spellings, and runner labels, pools and bare cluster ids;
    • TileRT decode-only and prefill-only bumps, and a TileRT point without PREFILL_IMAGE;
    • benchmark clients on another tag, an unrelated image, or another spelling, and a pinned frontend;
    • absolute, .. and unprefixed recipe paths, and a missing recipe.

A sweep that selects the B200 key without #3567's fix stops at setup with:

srt-slurm recipe preflight found 1 problem(s) affecting 13 point(s):
  srt-recipe=benchmarks/single_node/srt-slurm-recipes/dsv4/sglang/b200-fp4-mtp/agentic.yaml: Expected exactly one matching single-node SRT recipe; override_dep8_hicache_c128: Single-node SRT image: recipe/matrix 'lmsysorg/sglang:v0.5.19-cu130' != 'lmsysorg/sglang:v0.5.20-cu130'; ...
    - dsv4_tp8_conc1_kvnone_spec-draft_model on cluster:b200-nscale
    ...
    - dsv4_tp8_conc160_kvdram-hicache_spec-draft_model on cluster:b200-nscale | eval-only

Changes since the first review

  • e2e-tests.yml and profile.yml no longer import srtctl from the measured tree's submodule, which a PR could point at any code (Claude Code Review and the external review).
  • The check no longer depends on whether the measured tree contains preflight.py.
  • TileRT points are checked by role instead of skipped.
  • A reference that selects no variant is reported instead of falling back to base.
  • Recipe paths must stay inside their recipe trees.
  • benchmark.container_image must follow the point's image unless it names an unrelated client image.
  • A multi-node point without CONFIG_FILE is reported unless it is eval-only and sets EVAL_CONFIG_FILE (second review).
  • The first commit carries a bilingual body; the branch is rebased onto main 1f60c15b5, and the second-review fix is its own bilingual commit.

Validation

All results below are on this branch rebased onto main 1f60c15b5.

Notes for reviewers

  • Please merge fix(srt): align B200 and MI355X recipe images with master configs / fix(srt):对齐 B200 与 MI355X 配方镜像与主配置 #3567 first. Until it merges, a sweep that selects dsv4-fp4-b200-sglang-agentic-hicache-mtp fails at setup; it would fail at runtime anyway.
  • glm5.2-fp8-mi325x-sglang-agentic-mtp and dsv4-fp4-mi355x-sglang-disagg-agentic-umbp-dspark will fail at setup when a sweep selects them, until their recipes are fixed. Fixing the dspark client image changes what the benchmark runs, so it needs its own changelog entry and sweep. Only the points a sweep plans are checked, so neither blocks unrelated PRs.
  • single_node_environment mirrors how benchmark-tmpl.yml exports matrix fields to the job. A change to that mapping needs the same change here.
  • run-sweep only triggers on PRs that edit perf-changelog.yaml, so this PR's own checks don't run the new setup step. The replays above stand in for that.
  • There is no perf-changelog.yaml entry, because no benchmark config or recipe changes.
  • All changed files are owned by @SemiAnalysisAI/core.

AI model disclosure

Related Issue

No issue. Follow-up to #3567. Related: #3428, #3446.

Type of Change

  • Bug fix
  • New feature
  • Configuration change
  • Documentation update
  • Other (please describe)

Checklist

  • I have completed the AI model disclosure and kept it current
  • I have tested my changes locally
  • I have updated documentation if necessary
  • For every change that can affect benchmark performance and every recipe addition or modification, I have appended a new entry to the physical end of inferencex-e2e/perf-changelog.yaml and have not edited historical entries
  • Before merging via reuse, an authorized maintainer (OWNER/MEMBER/COLLABORATOR) has commented /use <run_id> (or the legacy /reuse-sweep-run) on this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the sweep label will no longer automatically kick off new sweeps. Remove and re-add the label to force one.
中文

改动说明

本 PR 是 #3567 的后续,两者是一个整体。#3567 修正了两个 container 与主配置 image 不一致的 srt-slurm 配方;本 PR 加上本可以发现这种不一致的检查:每次 sweep 都会把每个计划中的 srt-slurm 点绑定到它实际会启动的配方 variant,任何一个点绑定失败,就在派发任何任务之前停止。

问题: srt-slurm 点的镜像写在两个地方:主配置的 image(TileRT 还有 PREFILL_IMAGE),以及它所启动的配方里的各个 container。升级镜像必须同时修改两处(如 #3446),但在 benchmark 任务开始之前,没有任何检查比较这两处。

#3567 最初为此加了一个 pytest 扫描。Review 指出:它无法拦住独立的 run-sweep;它按配方比较镜像集合,而不是每个点实际选中的 variant;它没有检查 EVAL_CONFIG_FILE;并且在测试里固定了 checked-in 配置,违反 AGENTS.md 的测试规则。#3567 已移除该扫描,由本 PR 取代。

改动:

  1. Validator: infx/srt_slurm/preflight.py(python -m infx.srt_slurm.preflight)从 stdin 读入 matrix。所有 srt-slurm 点都能绑定时,原样输出到 stdout;否则每个问题只打印一次并列出受影响的点,然后以退出码 1 结束。
    • 单节点: 按 benchmark-tmpl.yml 为该点导出的环境变量,调用运行时自己的 select_recipe,必须在引擎、模型、镜像、精度、并行配置、GPU 数、投机解码、并发和 KV offloading 上恰好匹配一个 variant。由于逐点检查,被多个不同镜像的 key 共享的配方会分别对照每个 key 的镜像。
    • 多节点: 点必须设置 CONFIG_FILE,除非它是 eval-only 点并设置了 EVAL_CONFIG_FILE。这正是 lanes.config_file 会启动的配方;其他情况要到 GPU runner 上才会失败。validator 用 srtctl 自己的 override 展开逻辑,展开点所引用的每个 CONFIG_FILE 和 EVAL_CONFIG_FILE。展开出 0 个 variant 的引用会被报告,因为 srtctl 不会提交任何任务。在每个 variant 中:
      • model.container 和每个 roles.<role>.container 必须是该点的镜像(两种 registry 写法均可),或 configs/runners.yaml 为该 runner 所在 cluster 配置的 container alias。TileRT 点必须设置 PREFILL_IMAGE,其 prefill role 必须恰好是这个镜像,这是 launcher 为 prefill 暂存的唯一名字。
      • TileRT 的 frontend.container_image 或 benchmark.container_image 必须是 decode 镜像(或 alias)或 PREFILL_IMAGE。
      • 声明了 identity.container.image 时,它必须是该点的镜像。
    • Benchmark 客户端: 无论单节点还是多节点,benchmark.container_image 如果写的是该点镜像的另一个 tag,就会被报告,因为 srtctl 会拉取这个字面镜像,而不是使用暂存的镜像。无关的客户端镜像可以使用;其他 frontend 也可以固定自己的镜像,例如 ATOM agentic 配方为 atomesh frontend 固定了 rocm/atom-dev@sha256:f00a…。
    • 路径: 单节点的 srt-recipe 必须解析到 benchmarks/single_node/srt-slurm-recipes 之内;多节点引用必须是 recipes/ 路径,并解析到 benchmarks/multi_node/srt-slurm-recipes 之内,因此 .. 和绝对路径都会被拒绝。
    • Runner 配置: 只有 matrix 中有多节点点时才读取。如果本工具的 schema 无法读取它,validator 会明确说明并以退出码 1 结束。
  2. Workflow:
    • run-sweep.yml:setup 先初始化 srt-slurm submodule,在 benchmark_schema --plan 之后、ci_priority 之前运行 validator。validator 失败时 setup 失败,canary-select 和所有要求 setup 成功的 benchmark 任务都不会启动。
    • e2e-tests.yml(trusted-external-sweep.yml 和 claude.yml 也通过它派发)和 profile.yml:validator 及其 srtctl 都来自可信工具树(PRIORITY_ROOT 及其自己的 srt-slurm submodule),被测代码树只作为数据读取(--root "$MEASURED_ROOT")。该步骤始终运行。同一步骤中更早执行的 require_launcher 已经拒绝早于 Python launcher 的版本,而本工具无法读取 runners.yaml 的也正是这些版本。
  3. 测试: infx/tests/srt_slurm/test_preflight.py 按 AGENTS.md 的测试规则,用 36 个用例,只使用临时目录中的小型配方和 runner 配置,覆盖:
    • 通过 CLI 的单边镜像更新,以及只在有多节点点时才读取 runner 配置;
    • throughput 点和 eval-only 点在两个配置都没有、只有 EVAL_CONFIG_FILE、只有 CONFIG_FILE 时的情况;
    • 共享配方中的 variant 交换,以及 EVAL_CONFIG_FILE 不一致;
    • 不显式写 :base 时展开出 0 个 variant 的配方,以及缺少 model.container 的 variant;
    • 已配置和拼错的 alias、两种 registry 写法,以及 runner label、pool 和裸 cluster id;
    • TileRT 只升级 decode 或只升级 prefill,以及没有 PREFILL_IMAGE 的 TileRT 点;
    • benchmark 客户端使用另一个 tag、无关镜像或另一种写法,以及固定镜像的 frontend;
    • 绝对路径、.. 和缺少 recipes/ 前缀的配方路径,以及缺失的配方。

上方的示例输出,是一个选中 B200 key、但没有 #3567 修复的 sweep 在 setup 停止时打印的内容。

相对第一次 review 的改动:

  • e2e-tests.yml 和 profile.yml 不再从被测代码树的 submodule 导入 srtctl,因为 PR 可以让它指向任意代码(Claude Code Review 和外部 review 都指出了这一点)。
  • 检查不再取决于被测代码树中是否有 preflight.py。
  • TileRT 点改为按 role 检查,不再跳过。
  • 展开出 0 个 variant 的引用会被报告,不再退回检查 base。
  • 配方路径必须位于各自的配方目录内。
  • benchmark.container_image 必须跟随该点的镜像,除非它是无关的客户端镜像。
  • 没有 CONFIG_FILE 的多节点点会被报告,除非它是 eval-only 点并设置了 EVAL_CONFIG_FILE(第二次 review)。
  • 第一个 commit 的 body 为中英双语;branch 已 rebase 到 main 的 1f60c15b5,第二次 review 的修复是单独的双语 commit。

验证:

以下结果都基于 rebase 到 main 1f60c15b5 之后的本 branch。

审阅注意事项:

  • 请先合入 fix(srt): align B200 and MI355X recipe images with master configs / fix(srt):对齐 B200 与 MI355X 配方镜像与主配置 #3567。在它合入之前,选中 dsv4-fp4-b200-sglang-agentic-hicache-mtp 的 sweep 会在 setup 失败;这些 sweep 在运行时本来也会失败。
  • glm5.2-fp8-mi325x-sglang-agentic-mtp 和 dsv4-fp4-mi355x-sglang-disagg-agentic-umbp-dspark 在其配方修复之前,被 sweep 选中时会在 setup 失败。修正 dspark 的客户端镜像会改变 benchmark 实际运行的内容,因此需要单独的 changelog 记录和 sweep。validator 只检查 sweep 计划中的点,所以二者都不会挡住无关的 PR。
  • single_node_environment 复刻了 benchmark-tmpl.yml 把 matrix 字段导出给任务的方式;那边的映射改动时,这里也要同步修改。
  • run-sweep 只在修改 perf-changelog.yaml 的 PR 上触发,因此本 PR 自己的检查不会运行新的 setup 步骤,由上面的重放代替。
  • 没有 perf-changelog.yaml 记录,因为本 PR 不改任何 benchmark 配置或配方。
  • 所有改动文件都归 @SemiAnalysisAI/core。

AI 模型使用说明

关联 issue

无。#3567 的后续;相关 PR:#3428、#3446。

改动类型

新功能。

@chunfangamd
chunfangamd requested a review from a team October 1, 2026 05:51
Comment thread .github/workflows/e2e-tests.yml Outdated
Comment on lines +334 to +339
if [ -f "$PRIORITY_ROOT/infx/srt_slurm/preflight.py" ] && [ -f "$MEASURED_ROOT/infx/srt_slurm/preflight.py" ]; then
git -C "$MEASURED_ROOT" submodule update --init utils/srt-slurm
CONFIG_JSON=$(printf '%s' "$CONFIG_JSON" | env PYTHONPATH="$PRIORITY_ROOT:$MEASURED_ROOT/utils/srt-slurm/src" \
uv run --no-project --exclude-newer PT12H --python 3.12 --with pydantic --with pyyaml \
--with marshmallow --with marshmallow-dataclass --with ruamel.yaml --with requests \
python -P -m infx.srt_slurm.preflight --root "$MEASURED_ROOT")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 This runs the "trusted" preflight check with srtctl imported from the PR's own utils/srt-slurm submodule checkout, letting a malicious PR execute code inside the trusted-tooling process instead of only the sandboxed measured tree. check_multi_node -> selected_recipes does from srtctl.core.config import generate_override_configs, and PYTHONPATH is $PRIORITY_ROOT:$MEASURED_ROOT/utils/srt-slurm/src (e2e-tests.yml:336) while only $MEASURED_ROOT ever gets git submodule update --init utils/srt-slurm (line 335) — $PRIORITY_ROOT never does, so srtctl can only resolve from the PR-controlled submodule commit. -P doesn't block PYTHONPATH imports. Fix: give PRIORITY_ROOT its own trusted srt-slurm checkout (or vendor/pin the srtctl it imports) so the trusted interpreter never imports a package whose source lives only in the untrusted MEASURED_ROOT tree; same pattern in profile.yml:111-116.

Why this was flagged

The whole point of checking out .ci-priority/PRIORITY_ROOT separately from MEASURED_ROOT in this job is to run decision logic with code the PR cannot modify. This diff defeats that: infx.srt_slurm.preflight (loaded from PRIORITY_ROOT) calls infx.srt_slurm.synthetic_acceptance.selected_recipes, which lazily imports srtctl.core.config.generate_override_configs; that package is only ever present via $MEASURED_ROOT/utils/srt-slurm/src (e2e-tests.yml:335-336), which is the PR's own branch/submodule pointer. A PR that points its utils/srt-slurm submodule gitlink at a malicious commit gets that code executed inside the trusted PRIORITY_ROOT python process when get-jobs runs (same for profile.yml:111-116). python -P only disables automatic sys.path prepending, not PYTHONPATH, so this import is not blocked. The output of this step feeds CONFIG_JSON/job outputs that downstream benchmark-dispatch jobs (which do use secrets.INFERENCEX_OFFICIAL_RO_HF_TOKEN, secrets.MODAL_TOKEN_ID/SECRET) consume, so compromising this step can also poison what those privileged jobs run.

Verification: e2e-tests.yml:335 runs git -C "$MEASURED_ROOT" submodule update --init utils/srt-slurm (PR-controlled submodule), and :336 sets PYTHONPATH="$PRIORITY_ROOT:$MEASURED_ROOT/utils/srt-slurm/src", so srtctl resolves only from the measured tree.

chunfangamd and others added 2 commits October 2, 2026 05:59
…atch

run-sweep, e2e-tests and profile now pipe their matrix through
infx.srt_slurm.preflight after benchmark_schema. A single-node point must
select exactly one variant through the runtime's select_recipe. Every
CONFIG_FILE and EVAL_CONFIG_FILE recipe a multi-node point can launch must
select at least one variant, and its worker containers must resolve to the
images the launcher stages: the point's image or a cluster alias, and
PREFILL_IMAGE for a TileRT prefill role. A TileRT frontend or client must
run one of those images, and a benchmark client that names another tag of
the point's image is reported. Recipe paths must stay inside their recipe
trees. A mismatch fails setup before any canary or benchmark job is
dispatched.

In e2e-tests and profile, the validator and its srtctl come from the
trusted tree, which reads the measured tree only as data, and the check
always runs. require_launcher already rejects revisions older than the
Python launcher, which are also the revisions whose runner config this
tooling cannot read.

run-sweep、e2e-tests 和 profile 现在在 benchmark_schema 之后把 matrix 交给
infx.srt_slurm.preflight 检查。单节点点必须通过运行时的 select_recipe 恰好
匹配一个 variant。多节点点可能启动的每个 CONFIG_FILE 和 EVAL_CONFIG_FILE
配方必须至少选中一个 variant,其 worker container 必须解析到 launcher 暂存
的镜像:该点的镜像或 cluster alias;TileRT 的 prefill role 则必须是
PREFILL_IMAGE。TileRT 的 frontend 和客户端必须使用这两个镜像之一;benchmark
客户端如果写了该点镜像的另一个 tag,会被报告。配方路径必须位于各自的配方
目录内。任何不一致都会让 setup 在派发 canary 或 benchmark 任务之前失败。

在 e2e-tests 和 profile 中,validator 及其 srtctl 都来自可信代码树,被测代码
树只作为数据读取,并且检查始终运行。require_launcher 已经拒绝早于 Python
launcher 的版本,而本工具无法读取 runner 配置的正是这些版本。

Co-authored-by: Cursor <[email protected]>
… points

The srt-slurm launcher (lanes.config_file) launches EVAL_CONFIG_FILE only
for an eval-only point that sets it; every other multi-node point needs a
non-empty CONFIG_FILE and otherwise fails only on the GPU runner. The
preflight now reports such points before dispatch, and still checks every
recipe a point references.

srt-slurm launcher(lanes.config_file)只会为设置了 EVAL_CONFIG_FILE 的
eval-only 点启动该配方;其他多节点点都需要非空的 CONFIG_FILE,否则要到
GPU runner 上才会失败。preflight 现在会在派发前报告这类点,并继续检查点
所引用的每个配方。

Co-authored-by: Cursor <[email protected]>
@chunfangamd
chunfangamd force-pushed the chun/srt-recipe-preflight branch from eddfb6b to 5930300 Compare October 2, 2026 06:08

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant